Skip to content

feat(inspect): implement SnapshotsTable scanning#801

Open
WZhuo wants to merge 3 commits into
apache:mainfrom
WZhuo:inspect
Open

feat(inspect): implement SnapshotsTable scanning#801
WZhuo wants to merge 3 commits into
apache:mainfrom
WZhuo:inspect

Conversation

@WZhuo

@WZhuo WZhuo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Implement Scan() for SnapshotsTable, adding the core scanning infrastructure to MetadataTable and materializing snapshot rows into Arrow arrays via ArrowRowBuilder.

Changes

metadata_table.h / .cc

  • Add SnapshotSelection struct with snapshot_id, as_of_timestamp, ref_name optional fields for time-travel queries
  • Add Scan() virtual method returning Result<ArrowArray> with a convenience zero-arg overload
  • Add supports_time_travel() concrete method driven by kind(), matching Java's TIME_TRAVEL_TABLE_TYPES static set (currently both kSnapshots and kHistory return false)
  • Default Scan() returns NotSupported

snapshots_table.h / .cc

  • Declare and implement SnapshotsTable::Scan() — iterates source_table()->snapshots() and writes 6 columns (committed_at, snapshot_id, parent_id, operation, manifest_list, summary) via ArrowRowBuilder
  • committed_at converted to microseconds since epoch, matching Java's snap.timestampMillis() \* 1000

Tests

  • metadata_table_test_base.h — shared MetadataTableTestBase with MockFileIO + MockCatalog fixture, FinishAndImport(), MakeTestSnapshots(), MakeTableWithSnapshots() helpers
  • metadata_table_test.cc — 2 framework tests (FactoryRejectsNullSourceTable, SupportsTimeTravel)
  • snapshots_table_test.cc — 11 tests (Construct, SchemaMatchesIcebergSchema, 9 scan data tests matching Java TestDataTaskParser fixture)
  • history_table_test.cc — 1 schema test

@WZhuo
WZhuo marked this pull request as ready for review July 2, 2026 08:46
@WZhuo
WZhuo force-pushed the inspect branch 3 times, most recently from 28c4513 to a53f8ce Compare July 6, 2026 03:37
@WZhuo WZhuo changed the title feat(inspect): expand MetadataTable framework for all 16 metadata table types feat(inspect): implement SnapshotsTable scanning Jul 6, 2026
@WZhuo
WZhuo force-pushed the inspect branch 3 times, most recently from 9bc23e1 to 04b657f Compare July 7, 2026 02:04
- Add Scan() virtual method and Scan() convenience overload to MetadataTable
- Add SnapshotSelection struct for time-travel snapshot resolution
- Add supports_time_travel() concrete method driven by kind()
- Implement SnapshotsTable::Scan() to materialize snapshot rows via ArrowRowBuilder

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements initial metadata-table scanning support by adding a Scan() API to MetadataTable (with snapshot-selection parameters for future time-travel) and providing a concrete SnapshotsTable::Scan() implementation that materializes snapshot rows into Arrow arrays. The PR also restructures/extends the metadata-table test suite to validate schemas and snapshot scanning behavior.

Changes:

  • Added SnapshotSelection and a virtual MetadataTable::Scan() API (with a convenience overload) plus supports_time_travel().
  • Implemented SnapshotsTable::Scan() to emit snapshot rows (6 columns) via ArrowRowBuilder.
  • Added/expanded tests and wired new test sources into the metadata-table test target.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/iceberg/inspect/metadata_table.h Adds SnapshotSelection, Scan() API, and supports_time_travel() declaration/docs.
src/iceberg/inspect/metadata_table.cc Implements default Scan() + supports_time_travel() and wires factory for kinds.
src/iceberg/inspect/snapshots_table.h Declares SnapshotsTable::Scan() override.
src/iceberg/inspect/snapshots_table.cc Implements snapshot scanning into Arrow via ArrowRowBuilder.
src/iceberg/test/metadata_table_test.cc Simplifies base setup and adds SupportsTimeTravel test.
src/iceberg/test/metadata_table_test_base.h New shared fixture/helpers for metadata table tests.
src/iceberg/test/snapshots_table_test.cc New tests validating snapshots table construction/schema/scan output.
src/iceberg/test/history_table_test.cc New schema test for history table.
src/iceberg/test/CMakeLists.txt Adds new test sources to the metadata-table test target.

Comment thread src/iceberg/inspect/metadata_table.h Outdated
Comment thread src/iceberg/inspect/metadata_table.cc Outdated
Comment thread src/iceberg/test/metadata_table_test_base.h Outdated
Comment thread src/iceberg/inspect/metadata_table.h Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 05:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/iceberg/inspect/metadata_table.cc:38

  • supports_time_travel() is currently hard-coded to return false. That matches today’s two Kind values, but it’s easy to forget to update once additional metadata table kinds are added, and it doesn’t reflect the docstring/PR description that this is kind-driven. Consider switching on kind() and making the non-exhaustive case unreachable to keep future additions honest.
bool MetadataTable::supports_time_travel() const noexcept { return false; }

Comment thread src/iceberg/test/snapshots_table_test.cc
Comment thread src/iceberg/test/snapshots_table_test.cc
Copilot AI review requested due to automatic review settings July 23, 2026 01:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment on lines +79 to +87
static std::shared_ptr<::arrow::RecordBatch> FinishAndImport(ArrowArray array,
const Schema& schema) {
ArrowSchema c_schema{};
EXPECT_THAT(ToArrowSchema(schema, &c_schema), IsOk());
auto arrow_schema = ::arrow::ImportSchema(&c_schema).ValueOrDie();

// ImportRecordBatch takes ownership of the array and releases it.
return ::arrow::ImportRecordBatch(&array, arrow_schema).ValueOrDie();
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants